Always search games for new reviews #10

dfabulich · 2025-02-03T08:39:38Z

I'm feeling kinda confused, and, before we merge this, you might want to double-check my work.

First off, I'm quite surprised to see that iftechfoundation#1275 is happening in the main branch. I could've sworn I checked for this, but I can't see how this query could ever have possibly avoided a full scan. It's checking for reviews rows that have a non-null written review, but there's no key for that.

So I figured the best way to fix it would be to just have everybody use the new code that first searches for games with new reviews.

To actually make the reviews query benefit from this, I had to convert the $games_after_filtering post-query filtering code into SQL, adding a gameid in (...).

But then, I went in and explained the game-search query, just to be safe.

MariaDB [ifdb]> explain select
    ->               distinct games.id as id,
    ->                        games.title as title,
    ->                        games.author as author,
    ->                        games.desc as description,
    ->                        games.tags as tags,
    ->                        games.created as createdate,
    ->                        games.moddate as moddate,
    ->                        games.system as devsys,
    ->                        if (time(games.published) = '00:00:00',
    ->                            date_format(games.published, '%Y'),
    ->                            date_format(games.published, '%M %e, %Y'))
    ->                          as pubfmt,
    ->                        if (time(games.published) = '00:00:00',
    ->                            date_format(games.published, '%Y'),
    ->                            date_format(games.published, '%Y-%m-%d'))
    ->                          as published,
    ->                        date_format(games.published, '%Y') as pubyear,
    ->                        (games.coverart is not null) as hasart,
    ->                        avgRating as avgrating,
    ->                        numRatingsInAvg as ratingcnt,
    ->                        stdDevRating as ratingdev,
    ->                        numRatingsTotal,
    ->                        numMemberReviews,
    ->                        lastReviewDate,
    ->                        starsort,
    ->                        games.sort_title as sort_title,
    ->                        games.sort_author as sort_author,
    ->                        ifnull(games.published, '9999-12-31') as sort_pub,
    ->                        games.pagevsn,
    ->                        games.flags
    ->
    ->             from
    ->               games
    ->                           left join gameRatingsSandbox0_mv on games.id = gameid
    ->
    ->             where
    ->               lastReviewDate >= date_sub(now(), interval 365 day)
    ->
    ->
    ->
    ->             order by
    ->               lastReviewDate desc
    ->
    ->             limit 8;

+------+-------------+------------------------+--------+------------------------+----------------+---------+------------------------------------+------+------------------------------+
| id   | select_type | table                  | type   | possible_keys          | key            | key_len | ref                                | rows | Extra                        |
+------+-------------+------------------------+--------+------------------------+----------------+---------+------------------------------------+------+------------------------------+
|    1 | SIMPLE      | gameRatingsSandbox0_mv | range  | PRIMARY,lastReviewDate | lastReviewDate | 5       | NULL                               | 1217 | Using where; Using temporary |
|    1 | SIMPLE      | games                  | eq_ref | PRIMARY                | PRIMARY        | 130     | ifdb.gameRatingsSandbox0_mv.gameid | 1    |                              |
+------+-------------+------------------------+--------+------------------------+----------------+---------+------------------------------------+------+------------------------------+

That seems like way more rows than needed, (we need no more than 8 games) so I poked around with the query some more. It turns out that we don't need the lastreview: query filter at all!

That query term was my idea in my long post about performance… and it looks like the reason it was needed was the left join to gameRatingsSandbox0_mv. I merged iftechfoundation#1273 into this branch, and then, the lastreview: filter isn't needed.

I dunno. It seems to do the right thing for logged out users, and it worked when I set language:de as my custom search filter, even without a date limit.

The code was doing a `left join` (aka a `left outer join`) to `gameRatingsSandbox0_mv` unless `$browse` mode is enabled and you're using one of the specified sort orders, in which case it does a simple `join` (an `inner join`). I wrote this code in PR iftechfoundation#270, specifically in 3bd2c7a. But… why did I write that? Why didn't I just do an inner join in all cases? To be quite honest, I can't remember. I bet the issue was that `gameRatingsSandbox0_mv` didn't have exactly as many rows as the `games` table. (And I bet I was misusing `$browse` … it probably should have been `!$term`.) I believe that `gameRatingsSandbox0_mv` will always have a row for every game in the games table, now that we've merged iftechfoundation#1266, and so it should be safe to do an inner join in all cases.

Fixes iftechfoundation#1275

…ames-for-new-reviews

alice-blue · 2025-02-03T21:54:49Z

I don't entirely understand this PR, but one thing I've noticed so far is that on the allnew page, there is supposed to be a "games were filtered on this page" announcement at the bottom of the page when the filter is applied. In this branch, that announcement does not display. It checks for the variable $game_filter_was_applied to decide whether to display the announcement.

To reproduce, set up a filter, then on the home page, under new reviews, click "See the full list" to get to the allnew page.

alice-blue · 2025-02-03T22:58:15Z

www/newitems.php

+        $sortby = "recently_reviewed";
+        $games_limit_clause = "limit $reviews_limit";
+        $browse = 0;
+        [$game_rows_after_filtering] = doSearch($db, $term, $searchType, $sortby, $games_limit_clause, $browse);


In the case where no filter is set, I don't understand why this search is being run. Is it accomplishing anything other than recording the gameid of every game in the database? Wait...there's a limit, so I guess it's recording the gameids of like the 8 most recently reviewed games...?

Also, I haven't tested this, but if there are no spare gameids--if we're just grabbing the exact number of games we need--then what happens if the last 8 games that got reviewed were reviewed by someone on my mute list?

Indeed, it is recording the gameids of the 8 most recently reviewed games, including whatever custom search filter you may or may not have applied, but not incorporating muting, so, if someone you've muted posts 8 reviews, the home page reviews section could turn up blank. I think that's fine, because it's probably never going to happen.

Hmm. When I test this, and mute two of the front page reviewers, I seem to still be getting the same number of reviews on the front page (6 reviews) as are on the live IFDB site, and I still seem to be getting 50 reviews when I go to the "see full list" page. I'm not sure why....

alice-blue · 2025-02-03T23:20:25Z

It turns out that we don't need the lastreview: query filter at all!

Do you mean the thing that checks for reviews from the past 365 days if there's a filter set? Or something else?

We still end up sorting by last review regardless, right?

dfabulich · 2025-02-03T23:38:25Z

It turns out that we don't need the lastreview: query filter at all!

Do you mean the thing that checks for reviews from the past 365 days if there's a filter set? Or something else?

We still end up sorting by last review regardless, right?

Yes.

alice-blue · 2025-02-04T00:14:04Z

www/newitems.php

-                foreach ($game_rows_after_filtering as $game_row) {
-                    $gameids_after_filtering[] = $game_row['id'];
-                }
-                $game_filter_was_applied = 1;


I'm guessing that taking this out is why the announcement on the allnew page does not appear.

$game_filter_was_applied (or the equivalent) is actually returned by doSearch anyway--we're just not tracking it. This change will bring back the "games have been filtered" announcement on the allnew page.

alice-blue · 2025-02-04T00:35:03Z

I tried to create a PR to your PR and that didn't work, so I added a commit that I think fixes the problem with the disappearing announcement. You can tell me what you think, I guess.

I don't know if there's a way to get just the two things we need ($game_rows_after_filtering and $game_filter_was_applied) or if we have to get all of the values returned by doSearch. With my commit, it's getting them all.

Add comment explaining "game_rows_after_filtering"

dfabulich added 3 commits February 2, 2025 18:50

Always search games to find new reviews

37a1f48

Fixes iftechfoundation#1275

Merge branch 'searchutil-inner-join-gameratings' into always-search-g…

b29b7ea

…ames-for-new-reviews

alice-blue reviewed Feb 3, 2025

View reviewed changes

alice-blue reviewed Feb 4, 2025

View reviewed changes

newitems.php: get $game_filter_was_applied from doSearch

41094c4

$game_filter_was_applied (or the equivalent) is actually returned by doSearch anyway--we're just not tracking it. This change will bring back the "games have been filtered" announcement on the allnew page.

Update newitems.php

e9974e7

Add comment explaining "game_rows_after_filtering"

alice-blue merged commit c521685 into alice-blue:filter-games-version-3 Feb 4, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Always search games for new reviews #10

Always search games for new reviews #10

Uh oh!

dfabulich commented Feb 3, 2025

Uh oh!

alice-blue commented Feb 3, 2025 •

edited

Loading

Uh oh!

alice-blue Feb 3, 2025 •

edited

Loading

Uh oh!

alice-blue Feb 3, 2025

Uh oh!

dfabulich Feb 3, 2025

Uh oh!

alice-blue Feb 4, 2025

Uh oh!

alice-blue commented Feb 3, 2025

Uh oh!

dfabulich commented Feb 3, 2025

Uh oh!

alice-blue Feb 4, 2025

Uh oh!

alice-blue commented Feb 4, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Always search games for new reviews #10

Always search games for new reviews #10

Uh oh!

Conversation

dfabulich commented Feb 3, 2025

Uh oh!

alice-blue commented Feb 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alice-blue Feb 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alice-blue Feb 3, 2025

Choose a reason for hiding this comment

Uh oh!

dfabulich Feb 3, 2025

Choose a reason for hiding this comment

Uh oh!

alice-blue Feb 4, 2025

Choose a reason for hiding this comment

Uh oh!

alice-blue commented Feb 3, 2025

Uh oh!

dfabulich commented Feb 3, 2025

Uh oh!

alice-blue Feb 4, 2025

Choose a reason for hiding this comment

Uh oh!

alice-blue commented Feb 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

alice-blue commented Feb 3, 2025 •

edited

Loading

alice-blue Feb 3, 2025 •

edited

Loading

alice-blue commented Feb 4, 2025 •

edited

Loading